SK-2050: Add Set Token in Reveal Element #117
Conversation
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep findings: No issues found, Good to merge. |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep findings: No issues found, Good to merge. |
There was a problem hiding this comment.
Pull request overview
This PR adds the ability to update tokens in RevealElement components without remounting by exposing a setToken method via ref. Previously, changing a token required recreating the entire component.
Key changes:
- Added
setTokenmethod toRevealSkyflowElementclass for updating internal token state - Modified
RevealContainer.reveal()to fetch fresh tokens from elements before API calls - Exposed
setTokenviauseImperativeHandlein theRevealElementcomponent usingforwardRef
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/skyflow-error-code/index.ts | Added ELEMENT_NOT_FOUND error code for when element is not found in container |
| src/utils/logs/index.ts | Added error message template for ELEMENT_NOT_FOUND |
| src/core/RevealSkyflowElement/index.ts | Added setToken method to update internal token state |
| src/core/RevealContainer/index.ts | Modified reveal() to fetch fresh tokens from element instances before API call |
| src/components/RevealElement/index.tsx | Converted to forwardRef and exposed setToken via useImperativeHandle |
| tests/core/RevealContainer.test.js | Added test verifying reveal() uses updated token after setToken call |
| tests/components/components.test.js | Added tests for RevealElement rendering and setToken functionality via ref |
| tests/components/snapshots/components.test.js.snap | Added snapshot for RevealElement component rendering test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep findings: No issues found, Good to merge. |
Why
Outcome